Skip to content

Conversation

@a7vicky
Copy link
Member

@a7vicky a7vicky commented May 13, 2020

@openshift-ci-robot
Copy link

Hi @garbagego. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 13, 2020

// apiServerURL is a valid URI with scheme(http/https), address and
// port. apiServerURL can be used by components like the web console
// port(only if not using default port HTTP 80 and HTTP/TLS 443). apiServerURL can be used by components like the web console
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about ... and optionally a port (defaulting to 80 for http and 443 for https)?

You also have to generate the CRDs. make update or something like that. make help shows you the commands.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sttts Thanks for your suggestion.
I have updated the api doc and updated CRD by calling make update-codegen-crds

apiServerURL:
description: apiServerURL is a valid URI with scheme(http/https), address
and port. apiServerURL can be used by components like the web console
and port and optionally a port (defaulting to 80 for http and 443
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only one "and port"

@sttts
Copy link
Contributor

sttts commented Jun 2, 2020

/ok-to-test

@openshift-ci-robot openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 2, 2020
@sttts
Copy link
Contributor

sttts commented Jun 2, 2020

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 2, 2020
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

12 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

14 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@a7vicky
Copy link
Member Author

a7vicky commented Jun 4, 2020

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 4, 2020
@a7vicky a7vicky requested a review from sttts June 5, 2020 11:08
@sttts
Copy link
Contributor

sttts commented Jun 8, 2020

Need a swagger doc update.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 15, 2020
@a7vicky
Copy link
Member Author

a7vicky commented Jun 15, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 15, 2020
@a7vicky
Copy link
Member Author

a7vicky commented Jun 15, 2020

/retest

@sttts
Copy link
Contributor

sttts commented Jul 14, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: garbagego, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 5999429 into openshift:master Jul 14, 2020
wking added a commit to wking/openshift-api that referenced this pull request Jul 22, 2020
…L too

We've declared the default ports for apiServerURL since afb690a
(modified port details for APIServerURL, 2020-05-13, openshift#646).  Echo that
for apiServerInternalURL.  We don't have to call this out explicitly,
since the IANA URI scheme registry [1] links out to [2] for http and
[3] for https, which have:

  If the port subcomponent is empty or not given, TCP port 80 (the
  reserved port for WWW services) is the default.

and:

  All of the requirements listed above for the "http" scheme are also
  requirements for the "https" scheme, except that TCP port 443 is the
  default if the port subcomponent is empty or not given...

respectively.  But echoing those defaults locally doesn't hurt.

Autogenerated bumps via:

  $ hack/update-swagger-docs.sh
  $ make update-codegen-crds

[1]: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml
[2]: https://tools.ietf.org/html/rfc7230#section-2.7.1
[3]: https://tools.ietf.org/html/rfc7230#section-2.7.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants